Skip to content

chore(secrets): bump secretspec to v0.20, thread an audit reason, and stage an age-capable CLI (RIG-3320) - #916

Merged
trunk-io[bot] merged 9 commits into
mainfrom
compass-server/rig-3320-secretspec-v020
Sep 7, 2026
Merged

chore(secrets): bump secretspec to v0.20, thread an audit reason, and stage an age-capable CLI (RIG-3320)#916
trunk-io[bot] merged 9 commits into
mainfrom
compass-server/rig-3320-secretspec-v020

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Prerequisite for the RIG-2863 Layer-A boot-credential path, which needs an
age://-capable provider and a working resolver.Set.

The bump

secretspec-go v0.15.0 -> v0.20.0. The SDK API compass uses is unchanged, so
resolver.go compiles as-is; the version-pin drift guard moves to v0.20.0 and
re-cites the write-path evidence against v0.20 source (secrets.rs:4423-4427
piped-stdin branch and trim, :4430-4433 empty-value rejection). The
stdin/trim/empty-reject contract is byte-for-byte the same behavior it was at
v0.15, verified against the new source rather than assumed.

The CLI is staged, not built

The secrets write path shells a secretspec binary by name, and no closure
staged one, so age:// was unavailable in practice. age is a default-on cargo
feature of the crate, so a stock package already carries it — this adds bare
secretspec to the dev/CI shell rather than building anything. The pinned
channel ships 0.19.1, which has the age provider (0.17+).

Verified end to end against the staged binary: a write with the value on stdin
produces an age-encrypted file on disk, the value reads back intact, and the
plaintext is absent from the file.

The write path needed an audit reason

secretspec 0.17+ enforces a require_reason policy that defaults to agents
and refuses a set carrying no reason. The read path already threaded one
(Resolve -> WithReason), but the write path did not, so Set would have
failed closed against the new pin — including the master-key mint the Layer-A
path performs.

Set now carries a reason alongside the value, mirroring the read path, and
emits --reason before the set subcommand (it is a global flag). An empty
reason omits the flag entirely rather than passing --reason "", leaving the
CLI's own policy to accept or refuse the write. The secret value still never
appears in argv; it rides stdin only.

The argv-capture test now asserts the flag is present, positioned before the
subcommand, and that the value stays out of argv — so dropping or misordering
the flag fails the suite.

Ledger-impact: none
Refs RIG-2863

Co-authored-by: Matt Wilkinson matt@rigel.build

@trunk-io

trunk-io Bot commented Sep 6, 2026

Copy link
Copy Markdown

😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details.

@linear-code

linear-code Bot commented Sep 6, 2026

Copy link
Copy Markdown

RIG-3320

RIG-2863

@rigel-mintaka rigel-mintaka changed the title chore(secrets): bump secretspec to v0.20 and thread an audit reason through the write path (RIG-3320) chore(secrets): bump secretspec to v0.20, thread an audit reason, and stage an age-capable CLI (RIG-3320) Sep 6, 2026
@rigel-mintaka
rigel-mintaka changed the base branch from main to compass-server/ledger-id-collision September 6, 2026 05:18
@rigel-mintaka
rigel-mintaka force-pushed the compass-server/rig-3320-secretspec-v020 branch from 5819d35 to bbf59de Compare September 6, 2026 05:18
rigel-mintaka added a commit that referenced this pull request Sep 6, 2026
DL-330 was claimed by the merged apple-container macOS-runner record, so
Record A's row was a duplicate and would have red the fail-closed
design-ledger-gate on merge. DL-340 sits above every in-flight mint
(#927/#916 hold DL-333..337), not merely above main's tail — a tail check
cannot see a concurrently-open claim, which is how this class of collision
keeps recurring.

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@rigel-mintaka
rigel-mintaka force-pushed the compass-server/rig-3320-secretspec-v020 branch from bbf59de to e9d2d91 Compare September 6, 2026 15:28
… stage an age-capable CLI (RIG-3320)

The secrets write path shells the `secretspec` CLI (the Go SDK is read-shaped),
and three things were wrong with that seam.

**1. The SDK pin moved to v0.20.0.** `go build` needs no source change; the
write-path contract is unchanged and re-verified against v0.20.0 source
(`secrets.rs:4423-4427` for the piped-stdin branch and trim, `:4430-4433` for
empty-value rejection). The intentional tripwire `TestSecretSpecVersionPin`
moves with it.

**2. `require_reason` (0.17+) hard-fails a reasonless `set`.** The policy
defaults to `agents` and refuses a write with no reason, so the old argv broke
on the new CLI. `Set` now takes a `reason` and emits `--reason` before the
subcommand. An empty reason is **rejected up front** rather than omitting the
flag: the CLI's policy gates on agent-environment detection, so an omitted
reason makes the same write succeed on one host and fail on another, surfacing
to operators as a retryable provider fault. Screening it makes the failure a
deterministic caller error and makes the interface's "the reason travels with
every write" promise literally true.

**3. `Set` never told the CLI where the manifest was.** No `cmd.Dir`, no
`--file`, and no `secretspec.toml` is committed by design (the registry is the
source of truth), so every operator write failed `No secretspec.toml found in
current or any parent directory`. `Set` now generates a manifest declaring
exactly the name being written and passes it via the global `--file` flag — the
same explicit-manifest treatment `Resolve` already gave the read path. Verified
red/green against the real CLI from a manifest-less cwd: exit 1 without
`--file`, exit 0 with it, and an exact readback.

## Staging an age-capable CLI

The write path spawns `secretspec` by name, and nothing staged one, so `set` was
unreachable from the dev shell. A bare `secretspec` is not enough: this shell's
nixpkgs channel still resolves **0.14.0**, which has no `age` provider compiled
in at all and fails an encrypted-at-rest write with `Provider backend 'age' not
found` rather than degrading. It is therefore resolved from a second nixpkgs
input pinned in `devenv.lock`, carrying **0.20.0** — the same version as the SDK
pin, so the read path (SDK + native lib) and the write path (shelled CLI) move
together instead of skewing across an independent seam. Consumed as a dotted
attr outside the parsed `with pkgs` literal, matching `skopeo-nix2container`,
because the toolchain-parity gate resolves every bare attr in that list.

`age://` is proven end to end through the real dev shell: write with the value
on stdin (absent from argv), a 499-byte `-----BEGIN AGE ENCRYPTED FILE-----`
blob with the plaintext absent, and an exact readback.

## Drift guards

`TestSecretSpecVersionPin` only ever covered the SDK half; its doc now says so.
`TestSecretSpecCLIVersionFloor` covers the other half, asserting the staged
binary's version floor — the CLI decides whether `--reason` is accepted, whether
the policy exists, and whether `age` exists at all, and none of that is visible
to a go.mod pin. It skips when no binary is on PATH so hermetic runs stay green,
and it fails loudly against 0.14.0 with the provider diagnostic.

## Also

Both `vendorHash` literals that consume `go/go.mod` are refreshed — `flake.nix`
and `guest-image/default.nix`. Missing the second one failed `moon (nix)` on the
`compass-guestd` fixed-output derivation.

Three design-record sites that quote or prescribe the old 3-arg `Set` are
amended to the new signature, including the T2 provisioning instruction, which
now passes a concrete audit reason.

Ledger-impact: none.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@rigel-mintaka
rigel-mintaka force-pushed the compass-server/rig-3320-secretspec-v020 branch from e9d2d91 to 7a331f2 Compare September 6, 2026 16:31
@rigel-mintaka
rigel-mintaka changed the base branch from compass-server/ledger-id-collision to main September 6, 2026 16:32
rigel-mintaka and others added 8 commits September 6, 2026 12:54
…g=value argv (RIG-3320)

Review follow-ups on the secretspec v0.20 bump:

- `setArgs` now takes the resolved profile and emits `--profile`
  unconditionally. The generated manifest header and the argv are fed from one
  `resolvedProfile()` call, so the CLI cannot act under a different profile
  than the manifest declares by silently falling back to its own built-in
  default.
- Document that the joined `--flag=value` form is required rather than
  stylistic: the two-token form parses a leading-dash reason as the next flag
  and exits 2.
- Widen the `//nolint:gosec` justification to cover all three argv variables
  (name, reason, manifest path) instead of only the name.
- Drop the stale `secretspec-go v0.15.0` hashes left in `go.sum`.
- Cover the both-empty guard precedence in `Set`: the value guard fires before
  the reason guard, which `server.SetSecret` relies on when it maps a Set
  failure to `CodeUnavailable`.
- Refresh the design records line references onto the moved code.
Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
…r (RIG-3320)

setArgs used the joined --flag=value form for --reason and --file but
left --provider and --profile in the two-token form the same function's
comment declares unsafe. That rationale is a property of the value's
shape, not of which flag carries it: ValidateProfile admits a leading
dash and the provider URI is unvalidated, so a dash-leading value was
parsed as the next flag and the CLI exited 2 — surfacing through
SetSecret as a misleading retryable CodeUnavailable rather than a
config error. Join all four, and cover the operator-configured flags in
the hostile-input test that previously exercised only reason.

The version-floor test's failure message and devenv.yaml both cited a
0.15 age floor, contradicting the design record's 0.17. Upstream's
changelog puts age:// under 0.17.0, so the record was right and both
citations are corrected rather than the record.

Also refresh three stale artifacts: WithProfile's doc still promised the
SDK default an explicit --profile now precludes, the record's Load
citation still named the v0.15.0 pin (re-verified at v0.20.0
secretspec.go:293 — still no ctx, so the offload design stands), and the
audit reason was anonymous despite the handler holding the caller ID,
which is half the provenance the reason exists for.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
…it-reason contract (RIG-3320)

Round-3 review findings, all documentation/coverage — no behavior change.

- Design record: the round-3 commit added 5 lines to resolver.go and silently
  invalidated 23 file:line citations in the record it also edits. Re-point them
  all to the head tree, and fix the two SDK binding citations left at their
  v0.15.0 offsets inside the very sentence that was refreshed to v0.20.0
  (binding_cgo.go:30 -> :28, binding_purego.go:118 -> :142).
- resolver.go: the Set doc comment still illustrated the two-token
  '--provider <p> --profile <P>' form that the next sentence declares broken and
  that setArgs no longer emits. Show the joined form and the widened
  value-shape scope.
- secrets_service.go: lead the audit-reason comment with the structural
  guarantee (callerID is a bearer-resolved, server-minted hex account id, never
  a request field) and keep the CLI's JSON escaping as defense in depth.
- secrets_service_pgtest_test.go: the caller-bound audit reason was a new
  observable contract with no test. Bind the assertion to the authenticated
  user's account id and pin that the reason never carries the value.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
…tale v0.15 claims (RIG-3320)

Round-4 review findings. No behavior change.

- Design record: round 4 swept resolver.go citations only, but commit 1 shifted
  go/server/secrets_service.go by +10 lines, so four citations pointed at
  real-but-wrong code — including the F1 master-key guard's placement, which
  named an audit comment and a nil-resolver early return instead of the two
  resolver call sites. Re-point them, and audit EVERY citation into a file this
  PR touches rather than one filename: all 48 now resolve (resolver.go x34,
  secrets_service.go x12, go.mod x2).
- resolver.go + secrets_service.go: the `Delete` no-op was rationalized as
  "no CLI verb upstream". True at v0.15.0, false at this pin — `secretspec
  delete` shipped in 0.18 and is in the staged binary. Say the verb exists and
  that wiring it is a deliberate deferral (RIG-3436): it makes the operation
  destructive against a keyspace shared by default, so it needs its own
  F1-guard and ordering analysis. Same correction in the record.
- resolver.go: `defaultCLI` claimed the dev shell and the deployed image both
  stage the binary. Only the dev shell does; nothing stages it into the shipped
  artifact. Narrow the claim to what holds and point at RIG-3437.
- secrets_service_pgtest_test.go: hoist the secret literal so the value-absence
  assertion cannot silently stop testing anything when the input changes.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
…ot hand-counted (RIG-3320)

Round-4 review found a high. The previous commit re-pointed the
secrets_service.go citations and, in the same commit, edited resolver.go —
adding a line to the defaultCLI comment and three to the Delete comment —
without re-pointing the 34 resolver.go citations. 29 went stale, 7 of them onto
real-but-wrong executable code: resolver.go:171 is cited four times as the
uncancellable FFI `b.Load()` that anchors T2's whole bounded-offload design,
and it had come to resolve to `b = b.WithProfile(profile)`, a pure builder call
with no FFI and nothing to block on. An agent implementing from that reads the
offload machinery as unnecessary. Same class of break on the F1 master-key guard
citation: :218 landed on a comment, one line above the `resolver.Delete` call
it names.

The root cause is not carelessness on any one line, it is that the audit was
done by hand twice and both times scoped to the file I had most recently
thought about. So this pass computes it: diff each touched file against the
last pushed revision, build a parent-line -> head-line map from the matching
blocks, and rewrite every citation through the map. 31 refs moved. Then verify
in the other direction — every cited anchor is checked to still name the
construct the prose says it does (b.Load(), the Set/setArgs signatures,
exec.CommandContext, the F1 pre-call sites, go.mod:22), because in-range is not
the same as correct and only the second check would have caught this.

Also from the review:

- Two F2 passages said the repo "pins ... at v0.15.0" while citing
  go/go.mod:22, which this PR changes to v0.20.0 — the citation contradicted
  the sentence quoting it. Reframed as what it is: the pin as of writing, with
  the bump noted.
- Dropped the value-never-in-argv loop from TestSetArgs. setArgs takes no value
  parameter, so the string it scans for is never in scope and no mutation could
  redden it; it read like the guard while asserting nothing. The comment now
  points at TestSetFeedsValueOnStdinNeverArgv, which spawns a real process and
  checks the child's actual argv.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
…s too (RIG-3320)

Round-5 review. Last round's fix computed the line map correctly and then
applied it to exactly one document — the record this PR already edits. But the
citations of a shifted file are not confined to the record that happens to
discuss it: resolver.go moved +67 lines and devenv.nix +15, and 81 more
citations of those two files live in sixteen OTHER frozen records this PR never
touched. They were right at the merge base and wrong at head. Same defect as
round 4, one scope level out, and the previous commit message's claim to
'rewrite every citation through the map' described the map's construction
rather than where it got applied.

So the unit of the audit is now the SHIFTED FILE, not the edited document:
build a base->head line map for every code file this stack touches, then sweep
every markdown file in docs/ for citations into any of them. 27 resolver.go
refs across four records (native-client-mode, ownership-layer,
linear-agent-responder, forge-poll-driver) and 54 devenv.nix refs across twelve
more. The worst was the same b.Load() failure round 4 found: native-client-mode
cited resolver.go:162-164 for the WithProvider/WithProfile/Load chain, which at
head is a comment about temp-file cleanup.

Verified by text identity rather than by range, in both directions: for all 156
citation endpoints, the line the base document pointed at and the line the head
document points at are byte-identical. That check is what distinguishes a
correct citation from a merely in-range one, and it is the check that would have
caught both this and the round-4 high.

Two more from the review, both in the record this PR edits:

- The v0.15 reframing fixed the sentence quoting go.mod:22 but left the
  surrounding 'on the current pin' prose meaning v0.15, so each paragraph
  contradicted itself within four lines — stating the bump is done at v0.20.0
  while enumerating the v0.15 capability matrix and calling age:// gated. An
  agent implementing T0/T2's provider wiring could not tell whether the
  record's own ruled default was available. Both paragraphs now speak of v0.15
  in the past tense and state the prerequisite is met.
- The record's one inline spelling of the write command was the pre-PR shape,
  missing the two joined global flags this PR makes mandatory — it taught the
  argv form this PR's own test reddens.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
…s (RIG-3320)

Round-6 review. Two gaps in the previous sweep, both places its own stated
rule was not actually applied.

The commit message said the audit unit is the shifted FILE rather than the
edited document, then the sweep narrowed to markdown under docs/ — so eight
endpoints in Go source comments were never visited, including the same
b.Load() failure mode the last two rounds each called the worst case:
serve.go cited resolver.go:135-165 for the Resolve->b.Load() span, which at
head is a temp-file-cleanup comment and a defer os.Remove. A citation lives
wherever someone wrote it, and .go comments are a place people write them.
Fixed in serve.go and internal/stack/{spec,config,deps}.go; all four files are
untouched by this PR, so those citations were correct at the merge base and are
regressions this stack introduced.

The rewriter also only re-pointed the FIRST numeric component of a
comma-joined citation, leaving four trailing components at base values — e.g.
devenv.nix:328,383, where the 383 half is the --listen line the sentence is
actually about. The text-identity check missed it by comparing per-citation
rather than per-endpoint, which is the same shape of error: verifying the unit
you happened to iterate over instead of the unit that has to be right.

Both checks are now per-ENDPOINT and prefix-aware. Prefix-awareness matters
because a bare basename match would conflate root devenv.nix with
agent-image/devenv.nix, forks/*/default.nix and guest-image/default.nix, which
this PR does not touch — I verified the previous round shifted none of those.
All 15 endpoints verified by text identity: the line each citation named at the
merge base and the line it names at head are byte-identical.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
…ot just by it (RIG-3320)

Round 8 verified its rewrites by text identity: the line each citation named at
the merge base and the line it names at head are byte-identical. That claim was
true, and that is exactly the gap. Text identity proves a rewrite tracked the
line shift; it says nothing about whether the citation was correct BEFORE the
shift. Seven citations were already stale on main, so tracking the shift
faithfully preserved the staleness — and rule://planning-evidence asks for the
semantic property, that the cited line names the construct the prose claims.

Re-pointed against the construct each one claims, verified by asserting the
cited line contains the named flag/attr rather than by comparing base to head:

- go/internal/stack/spec.go:19  --runner-id: 293 (a GTK closure comment) ->
  498 for the spawn, :547 for the mint side, the two that must agree.
- go/internal/stack/spec.go:23  serverSpec: 199-205 (skopeo prose) -> 395-400,
  the --socket/--database/--listen/--tls-cert/--tls-key block it mirrors. This
  one round 8 did not touch and no round had flagged.
- go/internal/stack/spec.go:39  runnerSpec: 292-297 (pcClosure let-binding) ->
  497-502, the exec whose flags the comment enumerates.
- go/internal/stack/config.go:123  the 69-byte agent-socket suffix: 270-278
  (pkg-config `.pc` walk prose) -> 479-480, the sentence that states the path
  and the figure. Load-bearing: it grounds a 107-byte sun_path constant.
- go/internal/stack/deps.go:30  postgres ordering: 224-257 (cloud-hypervisor
  and the new secretspec block) -> :362 services.postgres and :386 the
  compass-server process, the two ends of the ordering.
- compass-local-dev/design.md:385  port doors: 275,280 (a `.pc` subdir comment
  and a bare `#`) -> 403,408, the ports.devhttp/ports.network allocations.
- compass-local-dev/design.md:567  runner.token and tls.crt: 343,398 and 347
  (RunSessions prose, container teardown prose) -> :548 mints the token and
  :496 reads it, :529 mints the cert and :500 trusts it.

Verified: all 16 endpoints assert-checked against the flag or attribute the
prose names, 0 mismatches. gofmt clean, go vet and go build pass, internal/stack
tests pass, markdownlint 199 files 0 errors.

Refs RIG-3320

Co-authored-by: Matt Wilkinson <matt@rigel.build>
@trunk-io
trunk-io Bot merged commit ef99bd1 into main Sep 7, 2026
14 checks passed
@trunk-io
trunk-io Bot deleted the compass-server/rig-3320-secretspec-v020 branch September 7, 2026 14:01
rigel-mintaka added a commit that referenced this pull request Sep 7, 2026
…symbols not lines (RIG-3326)

Two changes to the self-origin suppression record, both consequences of
rulings that landed after it was frozen.

1. The STATE actor carrier is RIG-3331's memo, not a field on the event.

This record froze the carrier as a new ForgeEvent.Actor field added by T0,
on the reasoning that ForgeEvent has no actor slot and COMMENT/REVIEW are
symmetric only because CommentRef holds theirs. Matt ruled the opposite on
2026-09-07 (RIG-3331 OQ-1): the actor travels in a durable, tenant-scoped
forge_state_transitions memo written at the write-path chokepoint and
consumed on match at the notify lane. A provider webhook is the forge's own
statement of what happened, and stamping a Compass field onto the struct
parsed from it invents an event the forge never sent.

So T0 adds no ForgeEvent field. The field would have shipped inert:
COMMENT/REVIEW resolve through CommentRef.Agent, OPENED through the DL-055
row, and STATE -- its only remaining client -- now resolves through the memo,
leaving no populator and no consumer (rule://no-inert-gating). The
fail-open interim is unchanged: a memo miss resolves no actor exactly as a
nil field did, so STATE delivers until RIG-3331's op is writing memos.

Amends the STATE row of the actor-source matrix, the carrier section, the T0
interface block, the T0 checklist entry, and DL-338's actor-source clause.
Also states the dependency direction explicitly: RIG-3331 is the
PREREQUISITE, this record is the consumer.

2. Symbol-plus-path citations.

Replaces every path:line citation in the record and in DL-338/DL-339 with
the symbol form. A line number is stale as soon as the file moves, and the
drift is observable on main: PR #916 had to chase six prose citations in a
merged record when resolver.go grew a few lines. All 104 were resolved to
their enclosing symbol against the source tree and verified present in the
cited file; the scrub was scoped to my own rows, leaving DL-064's citation
into the vendored fork tree alone rather than resolving it by basename into
an unrelated file.

Refs RIG-3326

Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka added a commit that referenced this pull request Sep 7, 2026
…symbols not lines (RIG-3326)

Two changes to the self-origin suppression record, both consequences of
rulings that landed after it was frozen.

1. The STATE actor carrier is RIG-3331's memo, not a field on the event.

This record froze the carrier as a new ForgeEvent.Actor field added by T0,
on the reasoning that ForgeEvent has no actor slot and COMMENT/REVIEW are
symmetric only because CommentRef holds theirs. Matt ruled the opposite on
2026-09-07 (RIG-3331 OQ-1): the actor travels in a durable, tenant-scoped
forge_state_transitions memo written at the write-path chokepoint and
consumed on match at the notify lane. A provider webhook is the forge's own
statement of what happened, and stamping a Compass field onto the struct
parsed from it invents an event the forge never sent.

So T0 adds no ForgeEvent field. The field would have shipped inert:
COMMENT/REVIEW resolve through CommentRef.Agent, OPENED through the DL-055
row, and STATE -- its only remaining client -- now resolves through the memo,
leaving no populator and no consumer (rule://no-inert-gating). The
fail-open interim is unchanged: a memo miss resolves no actor exactly as a
nil field did, so STATE delivers until RIG-3331's op is writing memos.

Amends the STATE row of the actor-source matrix, the carrier section, the T0
interface block, the T0 checklist entry, and DL-338's actor-source clause.
Also states the dependency direction explicitly: RIG-3331 is the
PREREQUISITE, this record is the consumer.

2. Symbol-plus-path citations.

Replaces every path:line citation in the record and in DL-338/DL-339 with
the symbol form. A line number is stale as soon as the file moves, and the
drift is observable on main: PR #916 had to chase six prose citations in a
merged record when resolver.go grew a few lines. All 104 were resolved to
their enclosing symbol against the source tree and verified present in the
cited file; the scrub was scoped to my own rows, leaving DL-064's citation
into the vendored fork tree alone rather than resolving it by basename into
an unrelated file.

Refs RIG-3326

Co-authored-by: Matt Wilkinson <matt@rigel.build>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants